fix(parser): preserve link destinations during streaming - #898
fix(parser): preserve link destinations during streaming#898jacobwisniewski wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: c8574bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The primary regression test doesn't actually pin the bug: its destination has an even number of underscores, so nothing is stripped on the old code either. It needs a destination with an odd underscore count to fail without the fix.
Reviewed changes — This is the initial review, covering the single commit cc1ecd0 on main.
- Streaming skip for complete links (
lib/src/parse.ts): in theoptimizeForStreamingdelimiter-counting scan, positions reported as a complete link by_skipLinkOrImageare now skipped so underscores/asterisks/tildes inside complete link and image destinations stop being counted as unmatched delimiters. _skipLinkOrImagereturn semantics (lib/src/parse.ts): the function (previously dead code with no callers) now returns the start positionifor shortcut and incomplete link/image constructs, so they are treated as unparseable text and still flow through the existing suppression logic.- Regression tests (
lib/src/parse.streaming.spec.ts): two tests for complete-link preservation and incomplete-link suppression. - Changeset (
.changeset/bright-links-preserve.md): patch formarkdown-to-jsx.
The fix logic itself is sound. I verified the underlying bug reproduces on the pre-change code (a destination like https://example.com/a_b comes out as https://example.com/ab in streaming mode) and is correctly preserved with this change. One test needs tightening before merge.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — This incremental re-review covers the new commit c8574bc landed since the prior Pullfrog review of cc1ecd0.
- Tightened regression test (
lib/src/parse.streaming.spec.ts): the complete-link destination changed fromgeoShape=KLUv_WCPBdUbAFa_sCUAy0w(two underscores, an even count that the old code never strips) togeoShape=KLUv_WCPBdUbAFa(one underscore). This directly resolves the prior must-address finding that the test did not fail without the fix.
The fix logic, incomplete-link suppression test, and changeset are unchanged from the prior review and remain sound. I verified the tightened test is now genuinely red/green: with the streaming skip block removed, KLUv_WCPBdUbAFa is corrupted to KLUvWCPBdUbAFa and the test fails; with the fix present it passes.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
1 participant
Summary
Problem
While markdown was being streamed, underscores inside a complete URL could be interpreted as unmatched formatting markers and removed from the destination. That changed otherwise valid generated URLs and could make them unusable.
Validation
bun test— 2,643 passedbun test --coverage— 92.85% functions, 94.50% linesbun run --filter markdown-to-jsx typecheck— passed